Repository navigation
DR-SEC-01g-G1: Denkraum ledger ownership primitive (option A) - #811
Conversation
Introduce a standalone Option-A ownership module with O_EXCL create, nonce-checked release, and fail-closed stale/malformed refusal before core/server wiring. Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: second process is refused while ownership is held Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: same-process second acquire with a different nonce is refused Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: stale heartbeat does not release ownership Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: malformed owner record fails closed Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: release with foreign nonce keeps the record Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: owner record is created exclusively with mode 0600 Co-authored-by: Cursor <cursoragent@cursor.com>
Merge Protections🟢 All 2 merge protections satisfied — ready to merge. Show 2 satisfied protections🟢 Paket-PR bringt seinen Bericht mitPackage branches (w<N>-, df<N>, ki-<N>, hq2-, dr-, z<N>-) must carry their report in the PR body, in a section that starts with "## Report".
🟢 🚦 Auto-queueWhen all merge protections are satisfied and these conditions match, this pull request will be queued automatically.
|
Overlapping release with the same nonce no longer deletes a successor's owner record: release is single-flight per nonce and unlinks only after a verified tombstone rename. Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: overlapping release cannot unlink a successor record Co-authored-by: Cursor <cursoragent@cursor.com>
…IP_IO Acquire and release normalize open/read/rename/unlink failures to a fixed OwnershipError code without attaching the raw cause or any path fragment. Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: filesystem failures use fixed diagnostics without paths Co-authored-by: Cursor <cursoragent@cursor.com>
…ilure When wx create succeeds but the initial write/flush fails, the creator unlinks its own incomplete record and returns a fixed OWNERSHIP_IO error. Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: failed write after create removes the half-written record Co-authored-by: Cursor <cursoragent@cursor.com>
…n close Register child cleanup immediately after spawn, join with a deadline on every path, and make the contender fixture exit non-zero if stdin closes without an explicit release. Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: fixture exits non-zero when stdin closes without release Co-authored-by: Cursor <cursoragent@cursor.com>
Pins the missing-record release path to OWNERSHIP_RELEASE_MISMATCH with path-free diagnostics (GLM F1). Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: release of a missing record is a fixed mismatch Co-authored-by: Cursor <cursoragent@cursor.com>
Unique per-release tombstones and exclusive link restore prevent a stale cross-instance release from overwriting a live successor (R811-O5). Test-First: tools/denkraum/store/ownership.test.mjs::DRSEC: cross-instance stale release never overwrites a live owner Co-authored-by: Cursor <cursoragent@cursor.com>
No-Test: documentation of the accepted API precondition and an honest test name (Elias E6b) Co-authored-by: Cursor <cursoragent@cursor.com>
No-Test: documentation of the accepted API precondition and an honest test name (Elias E6b) Co-authored-by: Cursor <cursoragent@cursor.com>
Cuarroc
added
the
review-ok
Codex Review SummaryThis comment shows the latest Codex review activity on this pull request.
ℹ️ About Codex in GitHubYour team has set up Codex to review pull requests in this repo. Reviews are triggered when you
Codex reacts with 👀 while any review is running, comments if it has suggestions, and reacts with 👍 once all reviews finish with no findings. |
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 4c56941acf
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| export class OwnershipError extends Error { | ||
| constructor(code) { super(code); this.name = 'OwnershipError'; this.code = code; } | ||
| } | ||
| export const ownerRecordPath = ledgerPath => `${ledgerPath}.owner`; |
There was a problem hiding this comment.
When the same existing ledger is supplied through a file symlink or hard-link alias, this derives different .owner paths from the two lexical names, so both acquireOwnership calls succeed and two servers are admitted. This is reachable through normal configuration because loadStartConfig preserves the lexical statePath after only using the physical path for validation; derive ownership from a stable physical identity or reject such aliases before G2 relies on this primitive.
Useful? React with 👍 / 👎.
| } | ||
| const record = parseRecord(raw); | ||
| if (!record || record.nonce !== nonce) throw new OwnershipError(OWNERSHIP_RELEASE_MISMATCH); | ||
| const tomb = `${path}.${nonce}.${randomBytes(8).toString('hex')}.tomb`; |
There was a problem hiding this comment.
For a valid ledger basename near the filesystem's component limit, acquisition succeeds because the owner file adds only .owner, but release appends another 55 characters for this tombstone and fails with OWNERSHIP_IO/ENAMETOOLONG, leaving the owner record permanently held. On a filesystem with a 255-byte component limit this already occurs for a 200-character basename, while the existing store can still write that ledger; use a bounded tombstone name in the same directory rather than extending the full owner filename.
Useful? React with 👍 / 👎.
| const nonce = randomBytes(16).toString('hex'); | ||
| const record = { nonce, pid: process.pid, heartbeatAt: new Date().toISOString() }; | ||
| let handle; | ||
| try { handle = await open(path, 'wx', 0o600); } |
There was a problem hiding this comment.
When the configured state file's parent directory does not yet exist, this open returns ENOENT and acquisition reports OWNERSHIP_IO. That state path is accepted by loadStartConfig, and DeskStore.change currently supports it by creating the parent recursively before the first write, so acquiring ownership before that write will make a fresh state location fail to start; create the parent during acquisition or require an explicit setup step before this call.
Useful? React with 👍 / 👎.
Merge Queue Status
This pull request spent 18 minutes 12 seconds in the queue, including 17 minutes 55 seconds running CI. Required conditions to merge
|
Es gibt jetzt eine eigene Sperrdatei neben dem Ledger: nur ein Schreibprozess darf sie zuerst anlegen. Ein zweiter Prozess oder eine alte/kaputte Sperre wird abgelehnt, ohne dass jemand die Sperre stillschweigend übernimmt. Nach Elias E6b ist „ein Nonce nur einmal freigeben“ dokumentierte API-Vorbedingung; Restrisiko und ehrlicher Testkommentar sind festgehalten — Delta-Check durch Root steht noch aus, noch nichts mergen.
Report
Package: DR-SEC-01g-G1 · Review tier: A (security/concurrency) · Model: Cursor auto
Base:
origin/main@15305c2e1eb503df4834f5cdce1a372f3c39964cHead:
4c56941acfbdc2d05f5641fd031a3e823b3764f6Goal / benefit: Standalone exclusive-writer ownership primitive (option A) before core/server wiring.
Parent / ticket / owner: DR-SEC-01g / Root decision 2026-10-09 option A / package G1
Native evidence: Cursor
CreateGoalis only for explicit user requests here; ticket goal from Root brief used as documented contract.Scope / limits: New module + tests + child fixture only; not wired into
core.mjs/server.mjs; no application-protection claim; size 300 lines (cap 300).Dependencies / acceptance: Design report Option A + G1 row;
node --test tools/denkraum/store/ownership.test.mjsexit 0;npm run test:denkraumexit 0;bash scripts/ci/gates.sh lane prepushexit 0.What changed
tools/denkraum/store/ownership.mjs: exclusive create (wx, mode 0600) of<ledger>.ownerwith nonce/PID/heartbeat; fixed diagnosticsOWNERSHIP_HELD,OWNERSHIP_MALFORMED,OWNERSHIP_RELEASE_MISMATCH,OWNERSHIP_RECOVERY_REFUSED(message equals code, no paths).recoverOwnershipalways refuses (G1 stub).ownership.test.mjs(real child process, no sleep >2s) andownership-contender.fixture.mjs.store/core.mjs,delivery.mjs,server.mjs,cli.mjs, orconfig.mjs.Architecture drift
Prompt-Log
Evidence
node --test tools/denkraum/store/ownership.test.mjs(ERR_MODULE_NOT_FOUND)node --test tools/denkraum/store/ownership.test.mjsnpm run test:denkraumbash scripts/ci/red-first.shbash scripts/ci/gates.sh lane prepushgit push -u origin cursor/dr-sec-01g-g1-ownershipgit ls-remote origin cursor/dr-sec-01g-g1-ownership→24f0b157042b17bbcaaa0df0d47446d349ef6a2dDiff size: 3 files, +225 / −0 (under 280-line cap).
NICHT ABGEDECKT
#[cfg(windows)]Rust tests and Windows clippy arms do not compile/run here; Windows verdict is the merge-queuegates (windows)run.0600assertion is POSIX-skipped on Windows (POSIX mode 0600 is not Windows ACL evidence); Windows ACL/handle/alias recovery remains for G6.Fix round 1 (NACHBESSERN)
Accepted round-1 findings with one commit each (test + fix). Assertion-RED recorded on head
24f0b157042bbefore the fixes (not import-error RED). R811-O3 (original Test-First was import-error RED at merge-base) is not repaired by rewriting history; the fix-round tests supply assertion RED on24f0b157.24f0b157(module/fixture restored; new tests kept)node --test --test-name-pattern '^DRSEC: overlapping release cannot unlink a successor record$' …node --test --test-name-pattern '^DRSEC: filesystem failures use fixed diagnostics without paths$' …node --test --test-name-pattern '^DRSEC: failed write after create removes the half-written record$' …node --test --test-name-pattern '^DRSEC: fixture exits non-zero when stdin closes without release$' …node --test --test-name-pattern '^DRSEC: release of a missing record is a fixed mismatch$' …24f0b157; coverage pin)cbadc7127484node --test tools/denkraum/store/ownership.test.mjsnode ~/pa-orch/reports/rv811-openai-probe.mjs(adapted outside repo)npm run test:denkraumbash scripts/ci/red-first.shbash scripts/ci/gates.sh lane prepushgit ls-remote origin cursor/dr-sec-01g-g1-ownership→cbadc7127484bb35230d820dda20fb62f8fc7946Diff size vs
origin/main: 3 files, +298 / −0 (≤300).Reviews and disposition
2bf5b08526da8d72fc225c8477c648e8eff88968(single-flight + tombstone)6eab0fe6ef80791343ca52755271a49eaeff843a(OWNERSHIP_IO, no cause/path)wxleaves empty lock896ff3f5a6a5883f13c930cf00ca2eeea1975d47(creator unlinks half-written)a2883e9ec44f04452a120912122a8a6655393248cbadc7127484bb35230d820dda20fb62f8fc7946(test pin)4c56941acfbdc2d05f5641fd031a3e823b3764f64c56941acfbdc2d05f5641fd031a3e823b3764f6(name kept for red-first trailer; clarifying comment only)4c56941)Fix round 3 (NACHBESSERN, only R811-O5)
Round 3 approved by Elias (E6) 2026-10-09; delta reviews OpenAI + Anthropic + GLM pending Root. Review: ausstehend (Stufe A).
cbadc7127484(hooks-only restore=rename):node --test --test-name-pattern '^DRSEC: cross-instance stale release never overwrites a live owner$' tools/denkraum/store/ownership.test.mjsnode --test tools/denkraum/store/ownership.test.mjs@57a0c811e0d0a4npm run test:denkraumbash scripts/ci/red-first.shbash scripts/ci/gates.sh lane prepushgit ls-remote origin cursor/dr-sec-01g-g1-ownership→57a0c811e0d0a41472d68d49e82640ac1d9e9116Diff size vs
origin/main: 3 files, +296 / −0 (≤300). Fix: unique per-release tombstone (nonce+random); on mismatch restore only via exclusivelink(neverrenameover a live path). Fixture trimmed for size budget; all behavioural assertions kept.Fix round E6b (NACHBESSERN, doc + test comment only)
Elias E6b (2026-10-09, „angenommen“): each release nonce may be released only once becomes a documented API precondition; residual risk of a second stale cross-instance release is accepted for G1; G2 (core) will enforce single release. No logic change.
4c56941acfbdc2d05f5641fd031a3e823b3764f64c56941acfbdc2d05f5641fd031a3e823b3764f6(honest clarifying comment; original test name kept because renaming brokered-firstagainst the earlierTest-First:trailer)node --test tools/denkraum/store/ownership.test.mjsbash scripts/ci/red-first.sh(after rename attempt)bash scripts/ci/red-first.sh@4c56941bash scripts/ci/gates.sh lane prepushgit ls-remote origin cursor/dr-sec-01g-g1-ownership→4c56941acfbdc2d05f5641fd031a3e823b3764f6Diff size vs
origin/main: 3 files, +300 / −0 (cap 300). Commits:582823e(precondition comment + rename attempt),4c56941(restore name for red-first). Review: delta check pending Root.Open points
4c56941acfbdc2on base15305c2; next owner Root for delta check, then G2 enforces single release when owners clear.Made with Cursor
Root-Stand nach Runde 2 (2026-10-09, head cbadc71)
releaseFlight); see O5OWNERSHIP_IO, no cause)review-pr811-openai-r2-cbadc7127484.md57a0c811e0d0a41472d68d49e82640ac1d9e9116Verdicts round 2: OpenAI reject (O5 high), Anthropic approve with conditions, GLM approve with conditions. E6 approved Round 3 (only O5); fixed at
57a0c811e0d0a4(size 296/300). Delta reviews OpenAI + Anthropic + GLM pending Root. Module is not wired into core/server yet. Review: ausstehend (Stufe A).Root: Runde 3 (head 57a0c81, 2026-10-09) — Nutzerentscheidung E6b
Round 3 approved by Elias (E6, „allen Empfehlungen folgen“), only for R811-O5. Fix 57a0c81 (unique tombstone + exclusive
linkrestore, 296/300).review-pr811-openai-final-57a0c811e0d0.mdreview-pr811-anthropic-final-57a0c811e0d0.mdOverwrite of a third owner's record: resolved (both). Double admission via two stale releases with the same nonce from two module instances: accepted residual risk per Elias E6b (documented precondition), G2 enforces — applied at
4c56941(API comment + honest test comment; rename reverted to keep red-first). Review: delta check pending Root.Root-Abnahme (head 4c56941, 2026-10-09 13:20 UTC)
Test-First:trailer pins the old name, red-first fails on a rename. Honest clarifying comment instead; the rename moves to G2 (withNo-Test:), together with core enforcement of single releaseNICHT ABGEDECKT: double admission through two stale releases of the same nonce from two module instances (accepted until G2); Windows half comes from the merge queue.